Skip to content

[codex] Implement managed execution env propagation#1178

Merged
Wirasm merged 2 commits intodevfrom
feature/managed-execution-env
Apr 13, 2026
Merged

[codex] Implement managed execution env propagation#1178
Wirasm merged 2 commits intodevfrom
feature/managed-execution-env

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented Apr 13, 2026

Summary

Describe this PR in 2-5 bullets:

  • Problem: managed per-project env vars were only injected into Claude workflow AI nodes, while Codex workflow nodes, bash/script DAG nodes, and direct orchestrator chat silently dropped them.
  • Why it matters: project-scoped secrets and config behaved inconsistently across execution surfaces, which broke the expectation that env configured once would work everywhere in project context.
  • What changed: threaded merged env vars into bash/script subprocess execution, added DAG capability warnings for env injection, made Codex construct per-call SDK instances when env is present, and forwarded codebase-scoped env vars through direct orchestrator chat.
  • What did not change (scope boundary): this does not make direct chat run in repo cwd; it only makes direct chat env-consistent when a conversation is codebase-scoped.

UX Journey

Before

User                   Archon                       Execution surface
────                   ──────                       ─────────────────
sets project env ───▶  stores repo/db env
runs Claude node ───▶  merges managed env ───────▶ Claude sees env
runs Codex node  ───▶  drops managed env  ───────▶ Codex misses env
runs bash/script ───▶  drops managed env  ───────▶ subprocess misses env
opens direct chat ─▶   drops managed env  ───────▶ provider misses env

After

User                   Archon                               Execution surface
────                   ──────                               ─────────────────
sets project env ───▶  stores repo/db env
runs Claude node ───▶  merges managed env ───────────────▶ Claude sees env
runs Codex node  ───▶  [builds per-call Codex with env] ─▶ Codex sees env
runs bash/script ───▶  [passes env to subprocess] ───────▶ subprocess sees env
opens direct chat ─▶   [merges repo+DB env in request] ──▶ provider sees env

Architecture Diagram

Before

WorkflowConfig/envVars ----> dag-executor ----> Claude provider
                                 |
                                 +----> Codex provider (no managed env path)
                                 +----> bash/script execFileAsync (no env path)

orchestrator-agent ----> provider requestOptions (assistantConfig only)
exec.ts ----> execFileAsync wrapper (no env option exposed)

After

WorkflowConfig/envVars ----> [~] dag-executor ===> [~] Codex provider
                                 |                     |
                                 |                     +===> per-call Codex(env)
                                 +===> [~] exec.ts/execFileAsync(env)
                                 +===> bash/script subprocesses

[~] orchestrator-agent ===> provider requestOptions(assistantConfig + env)

Connection inventory (list every module-to-module edge, mark changes):

From To Status Notes
packages/workflows/src/dag-executor.ts packages/git/src/exec.ts modified Bash/script nodes now pass merged subprocess env into execFileAsync.
packages/workflows/src/dag-executor.ts packages/providers/src/codex/provider.ts modified Workflow request options now rely on Codex env injection support and warn when unavailable.
packages/core/src/orchestrator/orchestrator-agent.ts packages/core/src/db/env-vars.ts new Direct chat now loads codebase-scoped DB env vars before sending provider requests.
packages/core/src/orchestrator/orchestrator-agent.ts packages/providers modified Direct chat request options now include merged env when present.
packages/providers/src/codex/provider.ts @openai/codex-sdk modified Provider now creates per-call Codex instances when env injection is requested.

Label Snapshot

  • Risk: risk: medium
  • Size: size: M
  • Scope: core|workflows|git|tests
  • Module: multi:managed-execution-env

Change Metadata

  • Change type: feature
  • Primary scope: multi

Linked Issue

Validation Evidence (required)

Commands and result summary:

bun run type-check
bun test packages/providers/src/codex/provider.test.ts \
  packages/workflows/src/dag-executor.test.ts \
  packages/core/src/orchestrator/orchestrator-agent.test.ts
  • Evidence provided (test/log/trace/screenshot): full workspace type-check passed; targeted provider/workflow/orchestrator regression tests passed.
  • If any command is intentionally skipped, explain why: full bun run lint, bun run format:check, and full bun run test were not rerun manually; staged-file eslint/prettier ran successfully in the commit hook.

Security Impact (required)

  • New permissions/capabilities? (Yes)
  • New external network calls? (No)
  • Secrets/tokens handling changed? (Yes)
  • File system access scope changed? (No)
  • If any Yes, describe risk and mitigation: managed env injection is intentionally expanded to additional execution surfaces. Risk is broader secret availability inside project-context executions; mitigation is explicit env merging only for configured project env, no new env sources, retained process cleanup, and capability/test coverage for each path.

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Database migration needed? (No)
  • If yes, exact upgrade steps:

Human Verification (required)

What was personally validated beyond CI:

  • Verified scenarios: workflow env passed to Claude request options, bash nodes, script nodes, Codex per-call construction, and orchestrator direct chat env merge.
  • Edge cases checked: empty env does not inject, Codex env path strips undefined process.env values to satisfy SDK typing, provider capability warnings fire for env injection.
  • What was not verified: direct orchestrator chat still does not run in repo cwd; this PR does not change that behavior.

Side Effects / Blast Radius (required)

  • Affected subsystems/workflows: workflow DAG execution, Codex provider lifecycle, direct orchestrator chat, subprocess execution wrapper, related tests.
  • Potential unintended effects: Codex requests with env now bypass the singleton and construct fresh SDK instances; subprocess env composition now explicitly includes managed env on bash/script nodes.
  • Guardrails/monitoring for early detection: targeted regression tests, type-check coverage, and DAG capability warnings when env injection is requested on unsupported providers.

Rollback Plan (required)

  • Fast rollback command/path: revert commit 2b8d4282 or revert this PR.
  • Feature flags or config toggles (if any): none.
  • Observable failure symptoms: managed env missing in Codex/bash/script/direct-chat project flows, or unexpected provider/subprocess failures when env is configured.

Risks and Mitigations

List real risks in this PR (or write None).

  • Risk: per-call Codex instantiation could change performance or lifecycle behavior for env-configured requests.
    • Mitigation: singleton path remains unchanged when no env is requested; targeted tests cover the env-specific constructor path.
  • Risk: expanded env injection increases secret exposure to more execution surfaces.
    • Mitigation: only project-scoped managed env is injected, merges remain explicit, and no new implicit env-loading behavior was introduced.
  • Risk: users may still expect direct chat to run fully inside the attached repo.
    • Mitigation: scope remains env consistency only; cwd behavior is unchanged and should be handled in a follow-up if desired.

Summary by CodeRabbit

Release Notes

  • New Features

    • Environment variables configured at the codebase level can now be injected into Codex provider execution and bash/script subprocess nodes, enabling consistent secret and configuration management across more execution surfaces.
  • Documentation

    • Updated documentation to reflect the expanded scope of environment variable injection across execution surfaces.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 13, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d410979d-dced-4b3d-bbe1-a8a0896bef85

📥 Commits

Reviewing files that changed from the base of the PR and between a8ac3f0 and cecfd0d.

📒 Files selected for processing (8)
  • CLAUDE.md
  • packages/core/src/orchestrator/orchestrator-agent.test.ts
  • packages/core/src/orchestrator/orchestrator-agent.ts
  • packages/git/src/exec.ts
  • packages/providers/src/codex/provider.test.ts
  • packages/providers/src/codex/provider.ts
  • packages/workflows/src/dag-executor.test.ts
  • packages/workflows/src/dag-executor.ts

📝 Walkthrough

Walkthrough

This PR implements managed execution environment injection across all execution surfaces with project context: orchestrator/direct chat, Codex and Claude providers, and bash/script workflow nodes. Changes include env-var loading and merging logic in the orchestrator agent, per-call Codex client instantiation when env is provided, subprocess environment propagation, and documentation clarification.

Changes

Cohort / File(s) Summary
Documentation
CLAUDE.md
Broadened codebase_env_vars scope from Claude SDK subprocess to include Codex, bash/script nodes, and direct chat when codebase-scoped. Updated workflow node YAML docs to reflect managed per-project env-var injection.
Orchestrator Agent
packages/core/src/orchestrator/orchestrator-agent.ts, packages/core/src/orchestrator/orchestrator-agent.test.ts
Added conditional loading of codebase-scoped environment variables via getCodebaseEnvVars, merging them with config-level envVars into requestOptions.env when conversation.codebase_id is present. Logs warning on load failure instead of failing request. New tests validate env merging, absent codebase handling, and failure scenarios.
Codex Provider
packages/providers/src/codex/provider.ts, packages/providers/src/codex/provider.test.ts
Introduced buildCodexEnv() to merge request env over process.env, and createCodexClient() to instantiate per-call Codex instances when request env is provided (falls back to singleton otherwise). Updated envInjection capability to true. Tests verify per-call vs. singleton behavior, env merging, and error wrapping.
Subprocess Execution
packages/git/src/exec.ts
Extended execFileAsync signature to accept optional env?: NodeJS.ProcessEnv parameter, enabling process environment control at call sites.
Workflow DAG Executor
packages/workflows/src/dag-executor.ts, packages/workflows/src/dag-executor.test.ts
Extended executeBashNode and executeScriptNode signatures to accept envVars, computing merged env and passing it to execFileAsync. Updated capability resolution to treat non-empty config.envVars as env injection support. Tests validate env propagation to subprocess calls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 The rabbit hops through execution paths,
Where env vars now flow with matched math—
Codex, Bash, and Chat align,
Each surface drinks from one design!
Managed secrets, clean and true,

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/managed-execution-env

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented Apr 13, 2026

PR Review Summary — Multi-Agent Analysis

Reviewed by 5 specialized agents: code-reviewer, docs-impact, silent-failure-hunter, pr-test-analyzer, code-simplifier.


Critical Issues (1 found)

Agent Issue Location
code-reviewer envInjection: true in CodexProvider makes the dag-executor capability-warning path permanently dead for Codex. The test at dag-executor.test.ts:2734 passes only because mockCodexCapabilities hard-codes envInjection: false, contradicting the real provider. Either the capability flag or the warning test is wrong — they cannot both be correct. Decision needed: if Codex supports env injection (it does via buildCodexEnv), remove the dead warning path and its test. If not yet reliable, revert envInjection to false. provider.ts:483, dag-executor.test.ts:120

Important Issues (4 found)

Agent Issue Location
code-reviewer buildCodexEnv allows requestEnv to silently override sensitive process-level env vars (e.g. OPENAI_API_KEY, PATH). Merge order is { ...process.env, ...requestEnv } — no denylist or warning. Document that this is intentional, or add a denylist for sensitive key prefixes. provider.ts:81-86
code-reviewer Per-call resolveCodexBinaryPath re-probes filesystem on every env-injected request — no caching, unlike the singleton path. Also, per-call Codex instance is never explicitly torn down (potential resource leak). provider.ts:504-510
silent-failure-hunter Per-call new Codex({...}) constructor is completely unguarded — no try/catch. Unlike the singleton path (getCodex) which has error recovery, a constructor failure produces a raw unclassified exception with no hint that env injection caused it. provider.ts:504-508
silent-failure-hunter getCodebaseEnvVars DB failure kills the entire message rather than gracefully degrading. A transient DB hiccup during env-var fetch prevents the AI query from ever being sent, with no specific log entry identifying the env-var load as the failure point. Consider wrapping with try/catch and falling back to {} with a warning. orchestrator-agent.ts:763-765

Documentation Issues (2 found)

Agent Issue Location
docs-impact Stale codebase_env_vars description: says "injected into Claude SDK subprocess env" — now incorrect since env vars also go to Codex and bash/script nodes. CLAUDE.md:389
docs-impact bash: and script: node docs don't mention they receive managed per-project env vars in their subprocess environment. Users authoring workflows have no way to know this without reading source. CLAUDE.md:689

Test Coverage Gaps (3 found)

Agent Rating Gap
pr-test-analyzer 8/10 buildCodexEnv merge order is untested — no test that requestEnv wins on collision or that process.env vars survive. Security-adjacent.
pr-test-analyzer 7/10 No test that getCodebaseEnvVars is NOT called when conversation has no codebase_id — the guard condition itself is unverified.
pr-test-analyzer 6/10 Codex singleton reuse not explicitly tested — no assertion that MockCodex is called only once across two sequential no-env calls.

Suggestions (4 found)

Agent Suggestion Location
code-simplifier Duplicated subprocessEnv construction — identical envVars && Object.keys(envVars).length > 0 ? { ...process.env, ...envVars } : undefined appears in both executeBashNode and executeScriptNode. Extract a small buildSubprocessEnv() helper. dag-executor.ts:1099, 1253
code-simplifier Conditional spread in orchestrator is unnecessarily indirect...(Object.keys(effectiveEnv).length > 0 ? { env: effectiveEnv } : {}) can be simplified to env: Object.keys(effectiveEnv).length > 0 ? effectiveEnv : undefined. orchestrator-agent.ts:766-770
code-simplifier Env guard pattern repeated 4x across 2 packages in slightly different forms. Standardizing on one form would reduce maintenance burden. Multiple files
silent-failure-hunter buildCodexEnv produces merged env with no debug logging of injected key names (not values). Makes debugging env-related Codex failures harder. provider.ts:81-86

Strengths

  • Well-structured PR with clear scope boundaries — env propagation only, no cwd behavior change
  • Bash/script subprocess env injection is clean and correct (merges only when non-empty)
  • Orchestrator correctly merges DB env vars over config env vars (DB wins on collision)
  • Test coverage for all four propagation paths (Codex, bash, script, orchestrator chat)
  • exec.ts change is minimal and backward-compatible (existing callers unaffected)
  • Retry and error paths in existing Codex provider code are well-tested

Verdict: NEEDS FIXES

Recommended actions (ordered by priority):

  1. Resolve the envInjection capability contradiction — the critical issue. If Codex genuinely supports env injection (it does), remove the dead warning path and update its test. Don't ship a test that validates dead code.
  2. Add try/catch around per-call new Codex({...}) — match the error handling quality of the singleton path.
  3. Consider graceful degradation for getCodebaseEnvVars DB failures — wrap with try/catch + warning fallback to {}.
  4. Add buildCodexEnv merge-order test — the highest-value missing test (security-adjacent).
  5. Update CLAUDE.md — fix stale codebase_env_vars description and document bash/script env propagation.
  6. Document or mitigate requestEnv overriding sensitive process vars — at minimum add a code comment explaining the design choice.

@Wirasm
Copy link
Copy Markdown
Collaborator Author

Wirasm commented Apr 13, 2026

Clarification on env override concern (item 2 under Important):

After discussion — the denylist suggestion is retracted. The merge order { ...process.env, ...requestEnv } where project env wins is the correct design. Users who configure per-project env vars expect them to take effect. Writing to codebase_env_vars or .archon/config.yaml requires the same access level as modifying the codebase itself, so this is an intentional override, not an injection vector. No code change needed here.

@Wirasm Wirasm marked this pull request as ready for review April 13, 2026 12:21
@Wirasm Wirasm merged commit bf20063 into dev Apr 13, 2026
4 checks passed
@Wirasm Wirasm deleted the feature/managed-execution-env branch April 13, 2026 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: managed execution env as a first-class, execution-consistent feature

1 participant